-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Smol+async global executor 1.80 dev #3791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Smol+async global executor 1.80 dev #3791
Conversation
# Conflicts: # .github/workflows/sqlx.yml
I would prefer to deprecate |
Sure, I did not remove |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same note as here: #3790 (review)
1f43f07
to
8d6377b
Compare
8c2c83c
to
9b0cebb
Compare
35237ac
to
c3e8f61
Compare
# Conflicts: # Cargo.toml
@martin-kolarik do you plan on addressing the CI failure and merge conflicts? |
I would like to address both. I will look at merge conflict first (today or on Monday) and we will see how tests changed. CI failure is linked to changes around |
# Conflicts: # Cargo.lock # Cargo.toml # rust-toolchain.toml # sqlx-cli/Cargo.toml # sqlx-core/Cargo.toml # sqlx-core/src/rt/mod.rs # sqlx-macros-core/src/lib.rs # sqlx-sqlite/src/connection/worker.rs
So, merged, fixed, and tests are running: the main issue of previous failures disappeared by itself, there certificate for localhost was expected. CI completion is prevented with error in build using minimal versions check, where |
Strictly speaking, this is a bug in The whole idea of the We could switch to I'd really like feedback from @iamjpotts here since he's the one who recommended adding the check to begin with (#3340). |
Yes, it is intended to be a correctness check on the declared minimums. From that standpoint the ci check is working as designed. The original issue was mentioned in #3118 (linked from #3340).
smol requires futures-lite 2.0, but in sqlx is optional. The fix is likely to be adding an explicit dependency on a transitive dependency, where the explicit dependency enforces the correct minimum. Possible workarounds to explore:
For either it would be a good idea to add a comment that the above is to resolve a problem with a transitive dependency (and which one, idk if it is |
Try to fix CI -Z minimal-versions check
I added dependency on |
This specific change lgtm though @abonander may have more fine grained feedback. |
async-global-executor
added as the fourth executor. The PR extends PR #3790.This is a breaking change --
async-global-executor
has MSRV 1.80, thereforesqlx
will have to be upgraded too.